Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(VSelect): port to v3 #14504

Merged
merged 45 commits into from
Feb 3, 2022
Merged

feat(VSelect): port to v3 #14504

merged 45 commits into from
Feb 3, 2022

Conversation

johnleider
Copy link
Member

@johnleider johnleider commented Dec 13, 2021

KNOWN ISSUES:

  • v-textarea auto-grow does not work
  • v-fileinput loses focus when opening folder selection dialog

Description

Ported v-select component using v-text-field as a baseline. Updated multiple functionality and design aspects of supporting input components (VInput, VField) to ensure a seamless transition between different options. Specific changes include:

  • VTextField

    • added hint and persistentHint props
    • added default slot before input element
  • VField

    • added model read functionality
    • added disabled / error state functionality
    • removed active prop (use modelValue)
    • removed VInput injections
  • VInput

    • removed focus, hint, and persistentHint props

Motivation and Context

Implement the v-select component

How Has This Been Tested?

n/a

Markup:

https://www.toptal.com/developers/hastebin/uvifabekey.xml

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any features but makes things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@johnleider johnleider added this to the v3.0.0 milestone Dec 13, 2021
@johnleider johnleider self-assigned this Dec 13, 2021
@johnleider johnleider force-pushed the feat/v3-select branch 2 times, most recently from 4d6f328 to aa88c96 Compare January 5, 2022 21:49
@johnleider johnleider marked this pull request as ready for review February 1, 2022 21:01
@jacekkarczmarczyk
Copy link
Member

  1. Should @click:prepend work at this stage? Because it doesn't form me
<template>
  <v-app>
    <v-main>
      <v-select :items="['a', 'b', 'c']" prepend-icon="mdi-vuetify" clearable @click:prepend="c" />
    </v-main>
  </v-app>
</template>

<script setup>
  const c = e => console.log(e)
</script>
  1. Not a fan of current transition for items menu, not only visually but also quick clicking on the option after opening the menu hides the menu without seleting the option. There's no menuProps any more to turn off transtiion, is there another way?

  2. Clearable icon shows only on hover, not sure if that's a good idea. Found the persistent-clear prop, I think it should be default true

  3. Keyboard navigation doesn't work (arrows and typing letters, only enter/space works for opening the menu)

@johnleider
Copy link
Member Author

  1. Should @click:prepend work at this stage? Because it doesn't form me

  2. Not a fan of current transition for items menu, not only visually but also quick clicking on the option after opening the menu hides the menu without seleting the option. There's no menuProps any more to turn off transtiion, is there another way?

  3. Clearable icon shows only on hover, not sure if that's a good idea. Found the persistent-clear prop, I think it should be default true

  4. Keyboard navigation doesn't work (arrows and typing letters, only enter/space works for opening the menu)

  1. Yes, I'll look into it
  2. This requires changes to how v-menu works, will be separate from this PR
  3. This matches the default text input functionality and I don't like boolean true default properties
  4. This is the responsibility of v-menu and v-list. It will come as a separate composable that's integrated into nested.

@jacekkarczmarczyk
Copy link
Member

  1. This matches the default text input functionality and I don't like boolean true default properties

Could be done with different property name, ex. show-clear-only-on-hover (not that exact name but you get the idea). I agree that that's not the scope of this PR though

@johnleider johnleider merged commit 23137ba into next Feb 3, 2022
@johnleider johnleider deleted the feat/v3-select branch February 3, 2022 15:54
inputRef.value?.focus()
}

if (!isFocused.value) isFocused.value = true
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants